refactor(stella-time): one home for the real time sources behind the Sleeper and Clock ports - #6488
Merged
Merged
Conversation
Contributor
|
SCR-003 DoD check waived by the |
macanderson
force-pushed
the
fix/6484-engine-clock-port
branch
from
September 10, 2026 07:50
4153a71 to
ec31417
Compare
macanderson
force-pushed
the
fix/6484-engine-clock-port
branch
2 times, most recently
from
September 10, 2026 17:48
4ed693c to
62937eb
Compare
macanderson
marked this pull request as ready for review
September 10, 2026 18:11
Contributor
There was a problem hiding this comment.
Sorry @macanderson, your pull request is larger than the review limit of 150,000 diff characters
macanderson
force-pushed
the
fix/6484-engine-clock-port
branch
from
September 10, 2026 18:34
62937eb to
2d1f68b
Compare
…verity advisories
… license check, as arenabench's once were
…Sleeper and Clock ports #6486 routed every instant, wait and timeout in stella-core through retry::Sleeper and left the real sources copied: a wall clock in stella-cli, stella-runtime and stella-serve, a Tokio sleeper in two of them, and a sleeper trait of stella-fleet's own with a third. stella-time now holds TokioSleeper, WallClock and MonotonicClock (which replaces the CLI's per-construction SystemClock), and ships PausedSleeper and NoopSleeper behind a test-util feature for the test sweep that follows. The copies are deleted; stella-fleet's monitor takes the engine's Sleeper. tests/one_home.rs reads the shipping tree and fails on any other copy. ADR 0042 records the design #6486 merged and where the sources live. Refs #6484
macanderson
force-pushed
the
fix/6484-engine-clock-port
branch
from
September 10, 2026 18:34
2d1f68b to
c6af5f7
Compare
This was referenced Sep 11, 2026
Merged
macanderson
added a commit
that referenced
this pull request
Sep 11, 2026
## What & why `main` is red on `lockfile-sync` and every `--locked` build (the canary's report is #6495). The `stella-time` crate landed in #6488 with its `Cargo.lock` entry at `0.9.416`, while the release sync in #6494 had already moved every workspace crate to `0.9.417`. Neither tree was wrong on its own; the composition is. This regenerates the lock, which changes exactly that one line. Refs #6495 ## The witness - [x] No witness needed (lockfile only) — because: `cargo metadata --format-version 1 --locked` fails on `main`'s tip and passes with this lock. That command is the `lockfile-sync` gate step. ## The gate - [x] `make guards-fast` (lockfile-sync included) passes locally - [ ] the rest is CI's ## Fix over file - [x] Extra fixes in this PR: none - [x] Nothing was deferred ## Anything reviewers should know? Carries `unblocks-main` so the red-main hold does not block its own repair, and `no-issue` because the canary's own run marks that issue resolved once `main` recovers; nothing here closes it.
4 tasks
macanderson
added a commit
that referenced
this pull request
Sep 11, 2026
…rp from the docs site (#6498) ## What & why Two follow-ups to #6488, each its own commit. - **`make core-no-io` told test code from shipping code by file name alone** (a `tests/` directory or a `tests.rs` file). A module declared under `#[cfg(test)]` by any other name was read as shipping code, which is why #6488's shared sleeper doubles had to be named `src/tests.rs`. The guard now reads every `#[cfg(test)] mod name;` line and skips the file or directory it names, at any depth. `scripts/test-core-no-io.sh` gains the case and its control (a plain `mod` line must still count), 29 cases in all. - **`sharp` is removed from the docs site instead of floored.** The site imports no `next/image`, so sharp was never loaded: inert weight from `next`'s optional dependencies that carried the advisory #6488 floored and fourteen LGPL-3.0 libvips tuples that `dependency-review` had to be told about by name on every `next` bump (the recurrence issue #2532 describes). The pnpm override drops it from the tree, its build-script approval goes with it, and the per-tuple exemption #6488 added to `dependency-review.yml` is deleted, since nothing is left to exempt. The `js-yaml` floor stays. Refs #6484. Closes nothing by design (`closes-nothing`). ## The witness - [x] This PR includes a witness test (fails on `main`, passes here) `scripts/test-core-no-io.sh`'s new case `a module named under #[cfg(test)] is not shipping code, by any name and at any depth` fails against `main`'s guard (the file it names reads `std::fs`) and passes here; its control `a module named by a plain mod line is shipping code` pins that the rule did not widen. For the sharp change the evidence is the lockfile: `sharp` appears once in `website/pnpm-lock.yaml`, in the override line, and `docs.yml`'s `typecheck + build` job is what proves `next build` does not need it. ## The gate - [x] `cargo fmt --check` (no Rust touched) - [x] clippy — no Rust touched - [x] tests — the guard harness ran locally, 29 passed; `make prose`, `make license-allowlist-parity` and `make action-pins` green - [x] Docs updated where behavior changed (the guard's own docstring and AGENTS.md's no-io paragraph name the new rule) - [x] CLA signed ## Fix over file - [x] Extra fixes in this PR: none beyond the two named - [x] Nothing was deferred ## Ground-rule check - [x] No I/O added to `stella-core` (not touched) - [x] No new outbound network calls - [x] No new cross-boundary serde types ## Anything reviewers should know? - If the docs site ever renders a `next/image`, restore a `sharp` floor in `website/pnpm-workspace.yaml` and put the tuples back in the workflow; the comment there says so. - The stacked doubles PR (#6491) was closed when #6488's branch was deleted on merge; it is re-opened against `main` separately and does not depend on this one. ## Summary by Sourcery Correct test-code classification in the core no-I/O check and remove unused Sharp dependencies from the documentation site. Bug Fixes: - Update the core no-I/O guard to exclude modules declared under `#[cfg(test)]` regardless of their file name or nesting depth, while preserving shipping-code detection for plain modules. Enhancements: - Remove the unused Sharp dependency from the documentation site and retain the existing js-yaml security floor. CI: - Remove Sharp and its platform-specific license exemptions from dependency-review configuration and its build-script approval. Documentation: - Document the expanded test-module exclusion rule in the core no-I/O guard guidance. Tests: - Add regression and control cases covering cfg-gated modules at multiple depths and plain modules.
macanderson
added a commit
that referenced
this pull request
Sep 11, 2026
…a-time (#6499) ## What & why The test-double half of #6484, on top of the merged `stella-time` PR (#6488). Re-opened: the first copy (#6491) was closed by GitHub when #6488's branch was deleted on merge. About thirty test files across `stella-core`, `stella-engine` and their `tests/` directories each wrote the same `Sleeper` double — a no-op one under three names, and one on tokio's paused clock — and each copy grew a `now()` body in #6486. Every one of them is deleted here. - Integration tests and `stella-engine` take `PausedSleeper` and `NoopSleeper` from `stella_time::test_util` through a dev-dependency on `stella-time` with `test-util` on. For `stella-core` that is a dependency cycle, which cargo allows; it is the tokio / tokio-test shape. - `stella-core`'s own unit tests keep one copy, `src/tests.rs`, because a lib's unit tests are a second build of the lib and a dev-dependency that links the lib implements the trait for the first. The compiler forces that copy (the first attempt at taking the crate's doubles from `stella-time` failed on every unit test with "multiple different versions of crate `stella_core`"). It is named `tests.rs` so `make core-no-io` reads it as test code, which is how that guard tells test code from shipping code. - The `one_home` witness in `stella-time` now scans test files too. Its kept list shrinks to the four doubles with a shape a shared one cannot have — the retry tests' recording sleeper, the monitor's advancing sleeper, the hanging sleeper, and `step/tests.rs`'s unpaused-time sleeper — plus the forced copy. Closes #6484 ## The witness - [x] This PR includes a witness test (fails on `main`, passes here) `crates/stella-time/tests/one_home.rs` reads every `.rs` under `crates/*/src` and `crates/*/tests`. On the base branch it fails on twenty-three files that each define a sleeper double; here it passes, and a second test fails if a kept one disappears. Locally: `stella-core`'s 893 lib tests and every integration test, `stella-engine`'s tests and the time crate's 6 + 3 tests pass. ## The gate - [x] `cargo fmt --check` - [ ] clippy over the workspace at `-D warnings` — CI; ran locally on `stella-core`, `stella-engine`, `stella-time` - [ ] the workspace test suite — CI - [x] Docs updated where behavior/flags changed (README, `--help`, doc comments) - [x] CLA signed - [x] `Closes #N` appears **both** above and as a commit trailer ## Fix over file - [x] Extra fixes in this PR: none beyond the sweep - [x] Nothing was deferred ## Ground-rule check - [x] No I/O added to `stella-core`; `src/tests.rs` is `#[cfg(test)]` and `make core-no-io` is green - [x] No new outbound network calls - [x] No new cross-boundary serde types ## Anything reviewers should know? - The rename `TokioSleeper` → `PausedSleeper` in the test tree is deliberate: the paused-clock double must not share a name with the real `stella_time::TokioSleeper`. - Once #6498 lands (the no-io guard reads `#[cfg(test)] mod` lines), `src/tests.rs` could take a more descriptive name; it is named for the guard's current file-name rule. ## Summary by Sourcery Consolidate test sleeper doubles in `stella-time` and enforce a single ownership location across the workspace. Enhancements: - Centralize the shared no-op and paused-clock sleeper doubles in `stella-time` and update `stella-core` and `stella-engine` tests to use them. - Retain only the specialized sleeper doubles that require unique behavior, including the compiler-required `stella-core` unit-test copy. - Extend the sleeper ownership witness to scan test sources and document the shared test utility arrangement. Build: - Add `stella-time` as a test-only dependency with the `test-util` feature where shared sleeper doubles are used. Documentation: - Update architecture and crate documentation to describe the shared sleeper test utilities and the `stella-core` unit-test exception. Tests: - Update the affected unit and integration tests to use the centralized sleeper doubles.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
#6486 routed every instant, wait and timeout in
stella-corethroughretry::Sleeper. It left the other half of the same defect in place: the real time sources are copied. A Unix-epoch wall clock lived three times (stella-cliWallClock,stella-runtimeHostClock,stella-serveWallClock); a Tokio sleeper twice (stella-cli,stella-serve); andstella-fleetkept aSleepertrait of its own with a thirdTokioSleeper.stella-servemay not linkstella-cliorstella-runtime, andstella-coremay not link the Tokio timer, so the copies had no home.This PR gives them one, and ships the two test doubles the follow-up sweep will move every test onto.
stella-timeholdsTokioSleeper(the engine's real sleeper andnow),WallClock(Unix epoch, for a stamp another process reads) andMonotonicClock(one origin per process, for a span compared as a number; replacesstella-cli's per-constructionSystemClock). The copies are deleted;stella-fleet's trait is gone and its monitor takes the engine's, re-exported under the old name. Justified under AGENTS.md § "When a new crate is justified" on two counts: it holds the effects the ports keep out ofstella-core, and it sits belowstella-serve. Exemplar: tokio / tokio-test.stella_time::test_utilshipsPausedSleeperandNoopSleeperbehind atest-utilfeature. Nothing takes them yet: the sweep that retires the ~30 per-file copies instella-coreandstella-engineis the stacked follow-up PR, kept separate so each diff stays under Sourcery's review limit and reads as one change.nowsits onSleeper, why the reading is anInstant) and where the sources and doubles live. fix(stella-core): read every instant, wait and timeout through the Sleeper port #6486 closed nothing and wrote no ADR; SCR-002 asks for one.Refs #6484 — the issue closes with the follow-up sweep, which lands its last two checklist items.
The witness
main, passes here)crates/stella-time/tests/one_home.rsreads every shipping.rsundercrates/*/src(test directories andtests.rsfiles skipped) and fails on anyimpl … Sleeper foroutsidestella-time, and on anystruct WallClock | HostClock | SystemClock | MonotonicClockoutside it. Theimpls that stay are named with reasons: two special-shape doubles (the retry tests' recording sleeper, the monitor's advancing sleeper) and five inline test doubles the follow-up sweep retires; a second test fails if a named one disappears, so the list cannot go stale. Onmainthe sleeper test fails onstella-cli/src/runtime.rsandstella-serve/src/remote.rs, and the clock test onstella-cli/src/runtime.rs(SystemClock,WallClock) andstella-runtime/src/wrapper/stamp.rs(HostClock);stella-fleet/src/monitor.rsis on the kept list for its advancing double, so its productionTokioSleeperwas reachable only through the trait it also deleted. Locally the time crate's 6 + 3 tests pass, andstella-core,stella-engine,stella-serve,stella-runtime,stella-fleetandstella-clitype-check with their tests.The gate
cargo fmt --check-D warnings— CI; ran locally, scoped and clean, on the six touched crates--help, doc comments)Closeshere by design: this PR advances The engine reads the clock itself in nineteen places, and three crates keep their own copy of the wall clock #6484 and the follow-up closes it (closes-nothing)Fix over file
main(sharp<0.35.4, GHSA-rgj7-g3m4-5g8c;js-yaml<4.3.2, GHSA-2883-xcg3-v3hh) are both transitive underwebsite/, sowebsite/pnpm-workspace.yamlraises thesharpfloor and adds ajs-yamlone, the way that file already handlespostcssandnanoid;js-yamltakes a caret because a bare floor resolves to 5.x, which fumadocs does not call.dependency-reviewre-surfaced sharp's fourteen LGPL-3.0 libvips tuples on that bump (not a required check). They are named inallow-dependencies-licenses, which is the immediate remedy issue dependency-review: the @img/sharp-* LGPL exemption cannot converge — the action has no purl globbing, so every new libvips tuple reds the gate #2532 records, with the reasoning in the workflow comment: the docs site is private, imports nonext/image, and ships nothing into either license track.AGENTS.mdcarried two crate counts ("Twenty-nine crates", "The other twenty-four crates") that a new crate makes wrong; both are now phrased without a number.Ground-rule check
stella-core; it is untouched except its READMEDeleted tests
Three tests in
stella-cli/src/runtime.rstested the clocks that module no longer defines, and each has a counterpart instella-time/src/lib.rs:system_clock_starts_near_zero_and_advances_monotonically→the_monotonic_clock_never_goes_backwards(plusevery_monotonic_clock_shares_one_origin, the property the old per-construction clock lacked)default_constructs_a_fresh_clock→ gone with the constructor;MonotonicClockis a unit structwall_clock_reads_epoch_milliseconds_not_a_process_origin→the_wall_clock_counts_from_the_unix_epochAnything reviewers should know?
u64readings ofClock. That design lost to fix(stella-core): read every instant, wait and timeout through the Sleeper port #6486 on the merge order and on the merits the ADR states (a double has to answernowandsleepfrom one timeline), and it was dropped rather than rebased over the merged one.stella-cli'sruntimemodule is now three re-exports andone_shot_budget_guard. TheSystemClockrename toMonotonicClockreachesfleet_cmd,agent/engine.rsand two test files, and nothing else.Summary by Sourcery
Centralize real time sources and shared sleeper doubles in
stella-timewhile preserving the existing time ports across all hosts.New Features:
stella-timecrate as the shared home for Tokio-backed sleeping, wall-clock timestamps, process-monotonic timing, and reusable test sleeper doubles.Bug Fixes:
Sleeperport.Enhancements:
stella-time.Build:
stella-timein the workspace and add it as a dependency for the affected host crates.CI:
Documentation:
Tests:
stella-timesource-layout witness tests.Chores: